Skip to content

HBASE-22567 - HBCK2 addMissingRegionsToMeta#18

Closed
wchevreuil wants to merge 10 commits into
apache:masterfrom
wchevreuil:HBASE-22567-rebased
Closed

HBASE-22567 - HBCK2 addMissingRegionsToMeta#18
wchevreuil wants to merge 10 commits into
apache:masterfrom
wchevreuil:HBASE-22567-rebased

Conversation

@wchevreuil
Copy link
Copy Markdown
Contributor

Rebased to current master and resolved conflicts.

@wchevreuil wchevreuil requested a review from saintstack August 29, 2019 15:47
@asf-ci
Copy link
Copy Markdown

asf-ci commented Aug 29, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/68/

@wchevreuil wchevreuil mentioned this pull request Aug 29, 2019
@busbey
Copy link
Copy Markdown
Contributor

busbey commented Aug 31, 2019

retest build

@asf-ci
Copy link
Copy Markdown

asf-ci commented Aug 31, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/73/

Comment thread hbase-hbck2/README.md
Comment thread hbase-hbck2/README.md Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/hbck2/meta/MetaFixer.java Outdated
@asf-ci
Copy link
Copy Markdown

asf-ci commented Sep 2, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/75/

Addressing following PR suggestions:
1) should we be throwing an exception to fail the tool run here?
2) could we add an option to fail the command if the disable fails? for folks who don't want to risk the overlap?
@asf-ci
Copy link
Copy Markdown

asf-ci commented Sep 2, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/76/

Comment thread hbase-hbck2/src/main/java/org/apache/hbase/hbck2/meta/MetaFixer.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/hbck2/meta/MetaFixer.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/hbck2/meta/MetaFixer.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/hbck2/meta/MetaFixer.java Outdated
admin.enableTable(tableName);
} catch (IOException e) {
LOG.debug("Failed enabling table {}. It might be that namespace table "
+ "region is also missing.\n"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these debug be under System.out.println? As this is important for the user that hbck2 did not leave the tables in the same state they were.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit of a dilema. On the cases I had been using this fix option, many often, namespace table was among the affected ones, which caused both disabling/enabling to fail, even though we didn't really care about it at this point. I was initially printing all errors, but that mislead operators to think the fix didn't work, when it actually did.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can understand your concern but these warnings should not be in debug, at least in WARN.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARN would be printed on console and cause same confusion as System.out.println. Moreover, if table enabling is failing for other reasons than namespace table not online, further problems would be manifesting on the cluster and those would need to be addressed on its own before operator can expect to run hbck2 properly.

Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/hbck2/meta/MetaFixer.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/hbck2/meta/MetaFixer.java Outdated
@asf-ci
Copy link
Copy Markdown

asf-ci commented Sep 3, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/77/

@asf-ci
Copy link
Copy Markdown

asf-ci commented Sep 3, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/78/

Comment thread hbase-hbck2/README.md Outdated
Comment thread hbase-hbck2/README.md
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/hbck2/meta/MetaFixer.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/hbck2/meta/MetaFixer.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java Outdated
Comment thread hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java Outdated
@asf-ci
Copy link
Copy Markdown

asf-ci commented Sep 4, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/81/

@asf-ci
Copy link
Copy Markdown

asf-ci commented Sep 4, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/82/

@asf-ci
Copy link
Copy Markdown

asf-ci commented Sep 4, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/83/

@asf-ci
Copy link
Copy Markdown

asf-ci commented Sep 4, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/84/

@asf-ci
Copy link
Copy Markdown

asf-ci commented Sep 4, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/85/

@wchevreuil wchevreuil self-assigned this Sep 5, 2019
@saintstack
Copy link
Copy Markdown
Contributor

Lets try and get this in @wchevreuil I want to make a release of the hbase-operator-tools. Need this. Once in, I'll some text to the fixit section in hbck2 README talking up this facility. Thanks boss.

@asf-ci
Copy link
Copy Markdown

asf-ci commented Sep 6, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/87/

@wchevreuil
Copy link
Copy Markdown
Contributor Author

Sure, let me get this committed, then.

@wchevreuil
Copy link
Copy Markdown
Contributor Author

I have this merged and conflicts resolved on my local master branch, but faced the problem raised on HBASE-22984. We need to get HBASE-22984 committed first, otherwise this one will fail hbck2 build.

@wchevreuil
Copy link
Copy Markdown
Contributor Author

Thanks for reviewing HBASE-22984 PR @busbey! Had it merged, let me now work on the commit for this one.

@wchevreuil
Copy link
Copy Markdown
Contributor Author

Committed to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants